fix(tower): create_sub_issue duplicate check fallback by name pattern#20
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughVersion 0.3.5 improves duplicate detection in sub-issue creation by adding name-based matching as a fallback. The ChangesDuplicate Detection Enhancement & Release Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/plane_conductor/mcp_tower.py`:
- Around line 699-734: The file fails ruff format --check; run the formatter and
commit the formatted file (e.g., run `ruff format
src/plane_conductor/mcp_tower.py`) so the block around root, root_name, seq,
identifier, title, items, existing and the DuplicateSubIssueError raise is
properly formatted; ensure wrapping, indentation, and trailing commas match
ruff's rules and then re-run the linter before pushing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f4603007-f309-4bb3-ab9b-083f317fea77
📒 Files selected for processing (3)
pyproject.tomlsrc/plane_conductor/mcp_tower.pytests/test_mcp_tower.py
The label-only duplicate check in create_sub_issue lets through duplicate sub-issues if the existing one has `labels: []` (e.g. dropped after create by some downstream workflow). This happened in production around 2026-05-09: two SPEC sub-issues (COINEX-48 / COINEX-50) were created under the same root after the original COINEX-38 — both had `labels: []` so the label match in the pre-condition missed them. Add a second-layer check on the canonical title pattern `<Role>: <root_name> (<identifier>)`. The title is deterministic from the role + root, so it survives any label-loss incident and acts as a defence-in-depth complement to the label check. Reorder the pre-condition: GET root before list_issues so the title is known when we filter. Existing label-check test updated to mock the extra GET; new test covers the empty-labels-but-matching-name case. Bumps to 0.3.5 (patch — behavioural fix, no API surface change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
90d9188 to
a5d9fc5
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plane_conductor/mcp_tower.py (1)
706-723: ⚡ Quick winName-fallback duplicate check can miss matches after root rename
On Line 722, the fallback uses full-title equality (
name == title). If labels are lost and the root issue name later changes, this check won’t match the old sub-issue title, so duplicates can still be created. Prefer matching stable parts (role prefix +(<identifier>)suffix) instead of fullroot_name.Proposed tweak
- title = f"{_role_display(role)}: {root_name} ({identifier})" + role_prefix = f"{_role_display(role)}:" + expected_suffix = f" ({identifier})" + title = f"{role_prefix} {root_name}{expected_suffix}" @@ - if str(i.get("parent") or "") == root_uuid - and (label_uuid in (i.get("labels") or []) or str(i.get("name") or "").strip() == title) + if str(i.get("parent") or "") == root_uuid + and ( + label_uuid in (i.get("labels") or []) + or ( + str(i.get("name") or "").strip().startswith(f"{role_prefix} ") + and str(i.get("name") or "").strip().endswith(expected_suffix) + ) + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plane_conductor/mcp_tower.py` around lines 706 - 723, The current duplicate check builds title and then matches full name equality, which fails if the root_name part later changes; update the existing list comprehension (the existing variable computation that filters plane.list_issues results) to detect fallback matches by checking for the stable pieces instead of full title equality — e.g., require the issue name to include the role prefix from _role_display(role) and to end with the identifier suffix "(<identifier>)" (or otherwise match a regex like ^<role_prefix>.*\(<identifier>\)$) while keeping the original label_uuid membership check; adjust references to title, root_name, identifier, and label_uuid in that comprehension accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/plane_conductor/mcp_tower.py`:
- Around line 706-723: The current duplicate check builds title and then matches
full name equality, which fails if the root_name part later changes; update the
existing list comprehension (the existing variable computation that filters
plane.list_issues results) to detect fallback matches by checking for the stable
pieces instead of full title equality — e.g., require the issue name to include
the role prefix from _role_display(role) and to end with the identifier suffix
"(<identifier>)" (or otherwise match a regex like
^<role_prefix>.*\(<identifier>\)$) while keeping the original label_uuid
membership check; adjust references to title, root_name, identifier, and
label_uuid in that comprehension accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c90e3ec-5f59-4ac8-9e56-115589bc06c4
📒 Files selected for processing (3)
pyproject.tomlsrc/plane_conductor/mcp_tower.pytests/test_mcp_tower.py
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_mcp_tower.py
CodeRabbit pre-merge check flagged docstring coverage at 75% (threshold 80%). Cover every helper in the file: registry helpers (label_uuid / artifact_label_uuid / slugs), mention rendering (_initiator_mention), HTTP client factory (_client_for), result projection (_summarize_issue), title helpers (_role_display / _strip_inline_tags), the asyncio.Lock factory for create_sub_issue, the JSON-line call-log emitter and its `mcp.tool` wrapper, the two nested `_comment_text` helpers in read_artifact / list_comments, and the entry-point helpers (_conductor_dir, _async_main). Each docstring captures the WHY rather than restating the signature. interrogate: 100.0% on src/plane_conductor/mcp_tower.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
create_sub_issueby canonical title pattern (<Role>: <root_name> (<identifier>)), complementing the existing label-based check.labels: [](cause not pinned down; the labels were either dropped after create or stripped by a later workflow).role+root_name, so it survives any label-loss incident.Test plan
pytest tests/test_mcp_tower.py -k create_sub_issue— 6/6 greenpytest tests/— full suite green (no regressions)DuplicateSubIssueErrorinstead of a second sub-issue.🤖 Generated with Claude Code
Summary by CodeRabbit